Migrate Connectivity Monitoring to NWPathMonitor on Apple Platforms#16112
Conversation
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
@ncooke3 Hi Nick, I’m still working on this PR. The replacement is less intuitive than I expected. I’ll ping you once the code is ready. :) |
|
./gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request migrates the network connectivity monitoring implementation for Apple platforms from the legacy SCNetworkReachability API to the modern NWPathMonitor API. The changes include updating build configurations, adding a new connectivity monitor implementation, and updating the Firestore client termination logic. I have identified a few minor improvements: some redundant includes should be removed, and the indentation of the empty block in the dispatch_sync call should be corrected for consistency.
3 participants
Design detail: go/firestore-nwpathmonitor
Summary
This PR migrates the network connectivity monitoring implementation for Apple platforms from the legacy
SCNetworkReachabilityAPI to the modernNWPathMonitorAPI. This modernizes the codebase and resolves potential issues with older network reachability APIs, while adding robust thread-safety guards during initialization and destruction.Key Changes
Firestore Core / Remote
NWPathMonitor: ReplacedSCNetworkReachabilitywithNWPathMonitorinConnectivityMonitorApple(connectivity_monitor_apple.mm).ConnectivityMonitorApplemust be both constructed and destructed on theAsyncQueueto prevent race conditions and use-after-free bugs with the monitor's handlers.FirestoreClient::TerminateInternal()in firestore_client.cc to reset theconnectivity_monitor_last, ensuring it is destroyed on theAsyncQueueafter all callback registrants are gone.Utilities
IsCurrentQueue()toAsyncQueueto allow components to verify they are running on the correct queue (used inConnectivityMonitorAppledestruction).Tests
FSTConnectivityMonitorTests.mmto verify thatUIApplicationWillEnterForegroundNotificationcorrectly triggers callbacks when the network is available.self.db = [self firestore];line inFIRIndexingTests.mmsetup.Build & Dependencies
Networkframework toFirebaseFirestoreInternal.podspecandPackage.swiftfor supported Apple platforms.Testing Done
FSTConnectivityMonitorTests.mmto verify foreground notification behavior.